Alert for ingestion processing - #2821
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
@@ Coverage Diff @@
## development/9.6 #2821 +/- ##
===================================================
- Coverage 76.51% 76.42% -0.09%
===================================================
Files 206 206
Lines 14450 14450
===================================================
- Hits 11057 11044 -13
- Misses 3383 3396 +13
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
bd6999f to
2f9c7f5
Compare
0e20eb4 to
a682dd2
Compare
570a524 to
24f4b57
Compare
bf0d146 to
4a35f25
Compare
Hello benzekrimaha,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
| # be resolved fails continuously. It is a handful of errors against the reads | ||
| # of every healthy source, so it only shows up on the absolute rate: the ratio | ||
| # alerts above stay far below their threshold when a single source is stuck. | ||
| - alert: IngestionProducerSourceSetupFailing |
There was a problem hiding this comment.
this alert is a sub-class of the previous one (IngestionProducerSourceErrorRate5Percent) :
- Why does it not report issues? Is this because of the threshold, or because one location's error may be lost in the other location's success, or because other calls are incorrectly reducing the ratio?
- Should we adjust the threshold instead? Or skip some ops which are not relevant to the alert? Or sum by bucket/location?
There was a problem hiding this comment.
For me it is the aggregation. A stuck source retries getRaftId every few seconds and those errors are drowned by getRaftLog successes from healthy sources. Lowering the ratio would noisy-alert on read errors and filtering the existing ratio to op="getRaftId" would fire, but then “error rate 5%” would mean setup failure...
The dedicated absolute rate on getRaftId, with for: 10m, is the smallest change that names this failure mode in my opinion
There was a problem hiding this comment.
A stuck source retries getRaftId every few seconds and those errors are drowned by getRaftLog successes from healthy sources.
- most often there are not many sources : like a few, and often just one. in any case, each source is one location, and we documented limit is 10 locations
- also in most cases, the locations are actually on the same server : so unlikely that one source fails while the others are fine
→ the aggregation could explain some corner case, but does not seem likely to explain the problem in every case?
→ the question still stands: we don't really care specifically that getRaftId failed, just that ingestion is not working. If the 5% alert is not working, it should be fixed, and we should avoid overlapping alerts : so we need to better understand what happens here before introducing a new alert which handles one case but leaves us "un-alerted" if it fails in the next line...
There was a problem hiding this comment.
I dropped IngestionProducerSourceSetupFailing and changed the 3% / 5% rules to sum by (op). A stuck setup is 100% errors on getRaftId; a later getRaftLog failure is the same alert on that op.
a682dd2 to
0c452ac
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
The following reviewers are expecting changes from the author, or must review again: |
|
Assigning @DarkIsDude for review since Mael is on PTO |
0c452ac to
3c1e419
Compare
3c1e419 to
922b7ff
Compare
A global error ratio hides a stuck getRaftId behind getRaftLog volume. Summing by op pages that setup failure and a later getRaftLog failure on the same 3% / 5% alerts. Issue: BB-605
The ingestion alerts had no render-and-test step. Cover a stuck getRaftId, a recovering setup, and a getRaftLog failure on the same 3% / 5% rules. Issue: BB-605
922b7ff to
4cb914f
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
|
/approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
The following branches will NOT be impacted:
This pull request does not target the following hotfix branch(es) so they
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
|
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue BB-605. Goodbye benzekrimaha. |
The ingestion producer already increments
s3_ingestion_source_operations_totalfor every source call, includinggetRaftIdat setup. The 3% / 5% alerts mixed everyopin one ratio, so a source that never attaches stayed quiet next togetRaftLogvolume, and there was no render-and-test coverage for these rulesIssue: BB-605